Skip to content

Add OpenTelemetry Javaagent framework - #1020

Merged
pivotal-david-osullivan merged 5 commits into
cloudfoundry:mainfrom
tylerbenson:tyler/otel
Dec 19, 2023
Merged

Add OpenTelemetry Javaagent framework#1020
pivotal-david-osullivan merged 5 commits into
cloudfoundry:mainfrom
tylerbenson:tyler/otel

Conversation

@tylerbenson

@tylerbenson tylerbenson commented Jun 27, 2023

Copy link
Copy Markdown
Contributor

This framework will wire up the OpenTelemetry Javaagent auto instrumentation. It leverages VCAP_SERVICES and the existence of a service binding with a specific name (otel-collector).

Borrows from @breedx-splk's work in #968.

Prerequisite: open-telemetry/opentelemetry-java-instrumentation#8800

Also, the otel-collector buildpack is not yet available, but perhaps the splunk collector could be utilized until it is.
I'm told that VMware is planning to include the collector in the base images, so perhaps this won't be required.

@tylerbenson

Copy link
Copy Markdown
Contributor Author

@tylerbenson
tylerbenson marked this pull request as ready for review August 16, 2023 20:42
@tylerbenson

Copy link
Copy Markdown
Contributor Author

@pivotal-david-osullivan could I get some eyes on this?
Thanks!


## User-Provided Service

Users are currently expected to provide their own "custom user provided service" (cups)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Users are currently expected to provide their own "custom user provided service" (cups)
Users are currently expected to provide their own "user provided service" (cups)

Could you also share an example user-provided-service command with sample required config?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I remove (cups) too? It won't match the abbreviation now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could just change 'provide' to 'create', because cups is the short command for create-user-provided-service 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this may be less relevant when CF has a collector bundled in the image VM, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this section a bit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the docs as they are currently, it seems a user would need to look up the properties required to connect the agent to a collector, which I think is required for this integration to be useable in this initial offering.

Therefore I was hoping to have a complete example of how to add the agent to an app, which gets config properties from the binding, and sends data to a user-defined collector. I think the last part is all that's left. I can then test the steps in my env, as a user.

Please let me know if I'm not correct about how this should work!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nrekretep I'm not a cloudfoundry user, so I could use your assistance. I'm not sure how to respond to David's request here. Do you think you could chime in with a more authoritative response?

@nrekretep nrekretep Dec 14, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If understanding correctly then the docs should contain a complete example of the necessary configuration for the user provided otel-collector service.

What needs to be added to the docs depends on which exporters and which protocol you want to use to send data to the collector.

Lets assume you want to just use the otlp exporter over grpc for metrics and traces and the collector endpoint uses tls and basic auth.

From my understanding the following properties need to be added to the user provided service:

{
  "otel.exporter.otlp.endpoint" : "https://my-collector-endpoint",
  "otel.exporter.otlp.headers" : "authorization=Basic SOMEBAS64STRING",
  "otel.exporter.otlp.protocol" : "grpc",
  "otel.metrics.exporter" : "otlp",
  "otel.traces.exporter" : "otlp",
}

We use this setup in CF, but we configure the otel agent via environment variables. Setting the java system properties as above should work, too.

Maybe adding a link to the complete set of configuration options for the java agent would help, too.

HTH

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be ideal @nrekretep - an example of basic/minimum config required for the user-service along with a link to the full options available.

@tylerbenson are you ok to add this to the docs part of your PR? Alternatively I could add this separately.

We're aiming to get this merged and into a release next week.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pivotal-david-osullivan I would really appreciate if you could take this over. Thanks!

Comment thread lib/java_buildpack/framework/opentelemetry_javaagent.rb
This framework will wire up the OpenTelemetry Javaagent auto instrumentation. It leverages VCAP_SERVICES and the existence of a service binding with a specific name (`otel-collector`).
Comment thread spec/java_buildpack/framework/open_telemtry_javaagent_spec.rb Outdated
Comment thread spec/java_buildpack/framework/open_telemtry_javaagent_spec.rb Outdated
Comment thread spec/java_buildpack/framework/open_telemtry_javaagent_spec.rb Outdated
Comment thread spec/java_buildpack/framework/open_telemtry_javaagent_spec.rb Outdated
Comment thread spec/java_buildpack/framework/open_telemtry_javaagent_spec.rb
Co-authored-by: David O'Sullivan <31728678+pivotal-david-osullivan@users.noreply.github.com>
@tylerbenson

Copy link
Copy Markdown
Contributor Author

@pivotal-david-osullivan Thanks for catching all those. I've applied all the changes.

@tylerbenson

Copy link
Copy Markdown
Contributor Author

@pivotal-david-osullivan following up here. Can we get this merged soon?

@nrekretep

Copy link
Copy Markdown

Hi there!
@pivotal-david-osullivan Is there any progress in getting this feature merged? It would really very helpful to have this in the buildpack.

@pivotal-david-osullivan
pivotal-david-osullivan merged commit 29cc897 into cloudfoundry:main Dec 19, 2023
@tylerbenson
tylerbenson deleted the tyler/otel branch December 20, 2023 20:03
ramonskie pushed a commit that referenced this pull request Dec 4, 2025
Add OpenTelemetry Javaagent framework
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants